fix: Resolve Web Crypto AES-GCM Map conversion bug during encryption#27
Conversation
WalkthroughReworked Python↔JavaScript Web Crypto AES‑GCM interop: algorithm objects are built via Changes
Sequence Diagram(s)sequenceDiagram
participant Py as Python (src/worker.py)
participant Bridge as Pyodide JS bridge
participant WC as WebCrypto (js.crypto)
Py->>Bridge: build algo via dict_converter=js.Object.fromEntries
Py->>Bridge: to_js(plaintext/ciphertext) (no create_pyproxies flag)
Py->>Bridge: allocate iv = new Uint8Array(12)
Bridge->>WC: crypto.getRandomValues(iv)
Bridge->>Py: iv -> bytes (store)
Py->>Bridge: importKey / encrypt / decrypt with algo + iv
Bridge->>WC: perform encrypt/decrypt
WC-->>Bridge: result (ciphertext/plaintext)
Bridge-->>Py: return bytes or raise error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worker.py`:
- Around line 166-172: The IV conversion in the decrypt path should be made
symmetrical to encrypt: instead of wrapping iv inline with to_js in the algo
creation, convert it once (e.g., create iv_array = to_js(iv)) and then pass
iv_array into algo creation so that the decrypt logic mirrors encrypt's use of a
JS Uint8Array; update the decrypt block that calls _derive_aes_key_bytes,
_import_aes_key and js.crypto.subtle.decrypt to use this iv_array when building
algo (referencing the existing names iv, to_js, algo, and
js.crypto.subtle.decrypt).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0ef6e43-ef37-4d4b-9003-786cd9580fca
📒 Files selected for processing (1)
src/worker.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/worker.py (1)
120-148:⚠️ Potential issue | 🟡 MinorTest coverage for the encryption functions would strengthen confidence in security and interoperability.
The
encrypt_aesanddecrypt_aesfunctions are critical to this application—they're used throughout the worker to protect sensitive user data (usernames, emails, roles, descriptions, locations). Currently, no tests exist in the repository. Since this is a Cloudflare Workers application running on Pyodide, consider adding integration tests that exercise these functions with various inputs (empty strings, Unicode, large payloads) to catch regressions in the Pyodide↔Web Crypto interop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 120 - 148, Add pytest integration tests that exercise encrypt_aes and decrypt_aes end-to-end: write tests that import encrypt_aes and decrypt_aes and verify round-trip correctness for empty string, multi-byte Unicode, large payloads, and various secret values; include a test that asserts a RuntimeError is raised and capture_exception is called when the Web Crypto interop fails (simulate by mocking _import_aes_key or js.crypto.subtle.encrypt to raise). Use mocks for Pyodide/js interop (patch js, pyodide.ffi.to_js, _derive_aes_key_bytes, and _import_aes_key) to run in CI without a browser, and add one integration test that runs against a real Pyodide/Web Crypto environment if available; ensure tests cover base64 output format ("v1:" prefix) and verify IV+ciphertext decoding during decryption.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/worker.py`:
- Around line 120-148: Add pytest integration tests that exercise encrypt_aes
and decrypt_aes end-to-end: write tests that import encrypt_aes and decrypt_aes
and verify round-trip correctness for empty string, multi-byte Unicode, large
payloads, and various secret values; include a test that asserts a RuntimeError
is raised and capture_exception is called when the Web Crypto interop fails
(simulate by mocking _import_aes_key or js.crypto.subtle.encrypt to raise). Use
mocks for Pyodide/js interop (patch js, pyodide.ffi.to_js,
_derive_aes_key_bytes, and _import_aes_key) to run in CI without a browser, and
add one integration test that runs against a real Pyodide/Web Crypto environment
if available; ensure tests cover base64 output format ("v1:" prefix) and verify
IV+ciphertext decoding during decryption.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: efa163d4-596a-422a-a422-7ee93f960f0e
📒 Files selected for processing (1)
src/worker.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/worker.py (1)
120-173:⚠️ Potential issue | 🟠 MajorAdd a regression test for the Web Crypto interop bridge.
The repository currently has no test coverage for
encrypt_aesanddecrypt_aes. Given that these functions power critical authentication endpoints (api_register,api_login, and others) and depend on Pyodide'sdict_converter=js.Object.fromEntriesto properly handle the Web Crypto API, a regression test is essential to prevent future breakage.A round-trip test exercising
encrypt_aes/decrypt_aesor anapi_registerflow test would provide the safest coverage for this environment-specific interop fix.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/worker.py` around lines 120 - 173, Add a regression test that performs a full round-trip using encrypt_aes and decrypt_aes (and optionally an api_register/api_login flow) to exercise the Pyodide Web Crypto interop path: call encrypt_aes(plaintext, secret) with a non-empty plaintext and secret, verify the result starts with "v1:", then pass the ciphertext into decrypt_aes(ciphertext, secret) and assert the original plaintext is returned; include tests for empty-string short-circuiting and for invalid/malformed ciphertext to assert "[decryption error]" is returned; ensure the test environment uses the same dict_converter=js.Object.fromEntries behavior (so the Web Crypto path that constructs algo via to_js({"name":"AES-GCM","iv": iv_array}, dict_converter=js.Object.fromEntries) is exercised) and run under the same Pyodide test harness used by other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/worker.py`:
- Around line 120-173: Add a regression test that performs a full round-trip
using encrypt_aes and decrypt_aes (and optionally an api_register/api_login
flow) to exercise the Pyodide Web Crypto interop path: call
encrypt_aes(plaintext, secret) with a non-empty plaintext and secret, verify the
result starts with "v1:", then pass the ciphertext into decrypt_aes(ciphertext,
secret) and assert the original plaintext is returned; include tests for
empty-string short-circuiting and for invalid/malformed ciphertext to assert
"[decryption error]" is returned; ensure the test environment uses the same
dict_converter=js.Object.fromEntries behavior (so the Web Crypto path that
constructs algo via to_js({"name":"AES-GCM","iv": iv_array},
dict_converter=js.Object.fromEntries) is exercised) and run under the same
Pyodide test harness used by other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 964d622b-b734-4fcd-a8a7-b6cdcbdb9b94
📒 Files selected for processing (1)
src/worker.py
ayesha1145
left a comment
ayesha1145
left a comment
There was a problem hiding this comment.
Great catch! The dict_converter=js.Object.fromEntries fix is exactly right — Pyodide marshals Python dicts as JS Maps by default, which breaks the Web Crypto API since it expects plain JS Objects for algorithm parameters. The IV handling improvement is also cleaner. LGTM ✅
2 participants
This PR resolves registration failures
(NotSupportedError: Unrecognized key import algorithm "undefined" requested.), occurred while registering a new user.By Explicitly converting Python dictionaries to standard JavaScript Objects using
dict_converter=js.Object.fromEntrieswhen passing cryptographic algorithm parameters tojs.crypto.subtle, preventing Pyodide from mistakenly converting them into Map objects.Summary
This PR fixes a Pyodide ↔ Web Crypto AES-GCM interop bug that caused registration failures with
NotSupportedError: Unrecognized key import algorithm "undefined". Pyodide was marshaling Python dicts as JS Maps, which resulted in an undefined algorithm being passed to the Web Crypto API. The change ensures algorithm parameters are converted to plain JS Objects so AES-256-GCM key import/encryption works correctly.Key changes (src/worker.py)
_import_aes_key
dict_converter=js.Object.fromEntrieswhen callingto_js, preventing Map conversion.create_pyproxies=Falsefor algorithm marshaling.encrypt_aes
Uint8Array(12)and fill it withjs.crypto.getRandomValues.dict_converter=js.Object.fromEntries(e.g.,{"name":"AES-GCM","iv": iv_array}).to_jswithout the oldercreate_pyproxies=Falseflags.decrypt_aes
to_jswithdict_converter=js.Object.fromEntries.to_js(simplified marshaling)."[decryption error]"on failure).Impact
undefinedalgorithm error.Lines changed: +19 / -16